-
Notifications
You must be signed in to change notification settings - Fork 84
EGNO #602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EGNO #602
Conversation
|
Hi! I know it is still draft but I would clean the GitHub history from polluted files. Just add one commit with the modified EGNN and EGNO block |
Hi @dario-coscia! Don't worry, I will take care of it! |
07eafce to
e725e30
Compare
|
@GiovanniCanali Ready for review? |
Almost there! I'll make it ready for review as soon as it is completed. |
e725e30 to
e019db6
Compare
|
Many thanks to @avisquid for the valuable contribution to this PR. Excellent work! |
e019db6 to
77a2317
Compare
77a2317 to
afc9fac
Compare
afc9fac to
ac542f6
Compare
dario-coscia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the code, and overall it looks solid. I have a few suggestions:
- Documentation: We should add more details about the input shapes for clarity.
- EGNN implementation with velocity: This part might need a closer look to ensure correctness.
- EGNO: Looks very good. We could experiment with adding more PINA blocks, especially considering the note about temporal spectral convolution.
pina/model/block/message_passing/equivariant_graph_neural_operator_block.py
Show resolved
Hide resolved
pina/model/block/message_passing/equivariant_graph_neural_operator_block.py
Show resolved
Hide resolved
ac542f6 to
0964918
Compare
Co-authored-by: avisquid <[email protected]>
0964918 to
0316208
Compare
Description
Implementation of the Equivariant Graph Neural Operator
This PR fixes #616
Checklist